Skip to content

fix(server): wake creator agent when board user rejects request_confirmation - #8516

Open
sunnystroeer wants to merge 1 commit into
paperclipai:masterfrom
sunnystroeer:sfs-100-fix-reject-wake
Open

fix(server): wake creator agent when board user rejects request_confirmation#8516
sunnystroeer wants to merge 1 commit into
paperclipai:masterfrom
sunnystroeer:sfs-100-fix-reject-wake

Conversation

@sunnystroeer

@sunnystroeer sunnystroeer commented Jun 23, 2026

Copy link
Copy Markdown

Fixes #8617

The reject path for request_confirmation / request_checkbox_confirmation silently dropped the continuation wake whenever the issue was assigned to a user at decision time (the typical "CEO hands review to the board" pattern). queueResolvedInteractionContinuationWakeup early-exits when assigneeAgentId is null, and the reject service never reassigned the issue back to the interaction's creating agent the way the accept path does. The accept path already handles this — reject was the asymmetric hole.

v2 (2026-08-08): refined implementation replacing the original submission —

  • Rename shouldReturnAcceptedConfirmationToCreatorAgentshouldReturnResolvedConfirmationToCreatorAgent (logic unchanged) so accept and reject share one predicate.
  • rejectRequestConfirmation now runs inside db.transaction, fetches live issue context, and when the predicate matches, reassigns the issue back to interaction.createdByAgentId with status todo (or blocked if it was already blocked) before returning a continuationIssue.
  • The /reject route uses continuationIssue ?? issue as the wake target, emits an issue.updated activity log with source: "request_confirmation_reject", and passes the reassigned issue into queueResolvedInteractionContinuationWakeup.
  • Covers both confirmation kinds (dispatcher routes both through rejectRequestConfirmation); suggest_tasks rejection is unaffected.

Tests: three new service-level tests (reassignment for both confirmation kinds + the agent-assigned regression case that must NOT reassign) and two new route-level tests asserting heartbeat.wakeup is called with the creator agent id — the assertion the original suite was missing. The existing wake_assignee_on_accept negative test still passes; that policy intentionally does not wake on reject.

🤖 Generated with Claude Code

@commitperclip

commitperclip Bot commented Jun 23, 2026

Copy link
Copy Markdown

✅ All checks passing — ready for Greptile review and maintainer approval.

— commitperclip

…firmation

The reject path for request_confirmation / request_checkbox_confirmation
silently dropped the continuation wake whenever the issue was assigned
to a user at decision time (the typical "CEO hands review to the board"
pattern). queueResolvedInteractionContinuationWakeup early-exits when
assigneeAgentId is null, and the reject service never reassigned the
issue back to the interaction's creating agent the way the accept path
does. The accept path already handles this — reject was the asymmetric
hole.

Mirror the accept-side reassignment in the reject path:

* Rename shouldReturnAcceptedConfirmationToCreatorAgent to
  shouldReturnResolvedConfirmationToCreatorAgent (logic unchanged) so
  both accept and reject share one predicate.
* Refactor rejectRequestConfirmation to run inside db.transaction, fetch
  the live issue context, and when the predicate matches, reassign the
  issue back to interaction.createdByAgentId with status todo (or blocked
  if it was already blocked) before returning a continuationIssue.
* Return { interaction, continuationIssue } from the reject path and
  update the /reject route to use continuationIssue ?? issue as the wake
  target, emit an issue.updated activity log with
  source: "request_confirmation_reject", and pass the reassigned issue
  into queueResolvedInteractionContinuationWakeup.

Covers both request_confirmation and request_checkbox_confirmation
because the dispatcher routes both through rejectRequestConfirmation.
suggest_tasks rejection is unaffected — it still returns the bare
interaction and the route handler distinguishes via in-narrowing.

Tests:

* services/issue-thread-interactions-service.test.ts — three new tests:
  reject reassigns a request_confirmation to the creator agent, ditto
  for request_checkbox_confirmation, and the regression case (issue
  still agent-assigned) does not reassign.
* __tests__/issue-thread-interaction-routes.test.ts — two new tests
  that exercise the full /reject route and assert
  heartbeat.wakeup is called with the creator agent id (the assertion
  the original suite was missing), one for each confirmation kind. The
  existing wake_assignee_on_accept negative test still passes — that
  policy intentionally does not wake on reject.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a stuck-issue bug in the board-review delegation pattern: when a board user rejects an agent-authored request_confirmation (or request_checkbox_confirmation) on a user-assigned issue, the creating agent was never woken because the wake helper keyed off assigneeAgentId (which was null). The fix mirrors the already-shipped accept path — rejectRequestConfirmation now runs inside a db.transaction, reassigns the issue back to the creator agent via issueService(db).update(..., tx), and returns { interaction, continuationIssue } so the route can pass the updated issue to queueResolvedInteractionContinuationWakeup.

  • Service layer: rejectRequestConfirmation wraps the rejection update and the optional issue reassignment in a single transaction; the else branch calls touchIssue(tx, ...) so the non-reassignment path remains atomic too.
  • Route layer: the /reject handler discriminates on "continuationIssue" in rejectResult, emits a secondary issue.updated activity log when reassignment occurred, and falls back to the original issue for the wake call when no reassignment was needed.
  • Tests: three new real-Postgres service tests and two new route tests cover the board-reject-wakes-agent path, the checkbox variant, and the already-agent-assigned no-op case.

Confidence Score: 5/5

Safe to merge — the behavioral change is tightly scoped to the board-reject-of-agent-confirmation case, and the common agent-assigned path is provably unchanged.

The reject path now mirrors the accept path end-to-end: same transaction shape, same issueService(db).update(..., tx) pattern, same continuationWakeIssue fallback, same activity log. assertTransition is permissive, issueService.update correctly uses the passed tx for all mutations, and touchIssue is preserved in the non-reassignment branch. Tests are thorough and cover both confirmation kinds plus the no-op guard.

No files require special attention.

Important Files Changed

Filename Overview
server/src/services/issue-thread-interactions.ts Renames helper to shouldReturnResolvedConfirmationToCreatorAgent, wraps rejectRequestConfirmation in a db.transaction, fetches issue context inside the transaction, calls issueService(db).update(..., tx) to reassign back to the creator agent, and returns { interaction, continuationIssue }. Correctly passes tx as the third arg so the issue update participates in the same transaction.
server/src/routes/issues.ts Updated /reject handler to discriminate on "continuationIssue" in rejectResult, derives continuationWakeIssue (falls back to original issue when null), emits an issue.updated activity log only when a reassignment occurred, and passes continuationWakeIssue to queueResolvedInteractionContinuationWakeup.
server/src/tests/issue-thread-interactions-service.test.ts Adds three new real-Postgres tests covering the board-reject-wakes-agent path for both confirmation kinds and the agent-assigned no-op case.
server/src/tests/issue-thread-interaction-routes.test.ts Updated existing test for new return shape and added two route tests verifying heartbeat wakeup and activity log for both confirmation kinds.

Reviews (3): Last reviewed commit: "chore: re-run PR checks after linking is..." | Re-trigger Greptile

Comment thread server/src/routes/issues.ts Outdated
Comment on lines +7001 to +7005
const rejectResult = await issueThreadInteractionService(db).rejectInteraction(issue, interactionId, req.body, {
agentId: actor.agentId,
userId: actor.actorType === "user" ? actor.actorId : null,
});
const interaction = "continuationIssue" in rejectResult ? rejectResult.interaction : rejectResult;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 PR description missing required template sections

The PR description does not follow the required template at .github/PULL_REQUEST_TEMPLATE.md. Specifically:

  • Thinking Path — required in blockquote style (5–8 steps tracing from project context down to this change). The "Why this shape" section is helpful but does not substitute for this.
  • Model Used — required section; not present anywhere in the description.
  • Risks — the template requires this section; it is absent. Even "Low risk" qualifies.
  • Internal ticket referencesSFS-96 and SFS-100 are instance-local ticket IDs. Per CONTRIBUTING.md → "No Internal Issue References", these must not appear in the PR body. Please restate the relevant context in plain English, or link the corresponding public GitHub issue instead.
  • PR checklist — not included.

Context Used: Contribution guidelines (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 7001-7005

Comment:
**PR description missing required template sections**

The PR description does not follow the required template at `.github/PULL_REQUEST_TEMPLATE.md`. Specifically:
- **Thinking Path** — required in blockquote style (5–8 steps tracing from project context down to this change). The "Why this shape" section is helpful but does not substitute for this.
- **Model Used** — required section; not present anywhere in the description.
- **Risks** — the template requires this section; it is absent. Even "Low risk" qualifies.
- **Internal ticket references**`SFS-96` and `SFS-100` are instance-local ticket IDs. Per `CONTRIBUTING.md` → "No Internal Issue References", these must not appear in the PR body. Please restate the relevant context in plain English, or link the corresponding public GitHub issue instead.
- **PR checklist** — not included.

**Context Used:** Contribution guidelines ([source](https://app.greptile.com/paperclip-org-3/-/custom-context?memory=a595932a-f6ed-448b-899b-5ccac43f9148))

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread server/src/routes/issues.ts Outdated
Comment on lines +7005 to +7006
const interaction = "continuationIssue" in rejectResult ? rejectResult.interaction : rejectResult;
const continuationIssue = "continuationIssue" in rejectResult ? rejectResult.continuationIssue : null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inconsistent rejectInteraction return shapes require a runtime discriminant

rejectSuggestedTasks returns a bare IssueThreadInteraction while rejectRequestConfirmation returns { interaction, continuationIssue }, forcing callers to use "continuationIssue" in rejectResult to tell the two apart. This discriminant is fragile: if IssueThreadInteraction ever gains a continuationIssue property the branch silently misroutes. Normalizing rejectSuggestedTasks to also return { interaction: hydrateInteraction(updated), continuationIssue: null } would eliminate the union and let callers always destructure the same shape.

Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 7005-7006

Comment:
**Inconsistent `rejectInteraction` return shapes require a runtime discriminant**

`rejectSuggestedTasks` returns a bare `IssueThreadInteraction` while `rejectRequestConfirmation` returns `{ interaction, continuationIssue }`, forcing callers to use `"continuationIssue" in rejectResult` to tell the two apart. This discriminant is fragile: if `IssueThreadInteraction` ever gains a `continuationIssue` property the branch silently misroutes. Normalizing `rejectSuggestedTasks` to also return `{ interaction: hydrateInteraction(updated), continuationIssue: null }` would eliminate the union and let callers always destructure the same shape.

How can I resolve this? If you propose a fix, please make it concise.

cryppadotta added a commit that referenced this pull request Aug 5, 2026
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Issue thread confirmations can pause an issue until a board user
makes a decision
> - Atomic checkout is the only supported transition into `in_progress`
> - An accepted confirmation left a creator-owned issue in `in_review`
while it started a continuation worker
> - The worker could run without the normal checkout state transition
> - This pull request returns that narrow review state to `todo` before
it queues the continuation wake
> - The benefit is that the worker can check out the issue and move it
to `in_progress` through the normal atomic path

## Linked Issues or Issue Description

No matching public GitHub issue exists. The following pull requests are
related but do not fix this case:

- Refs #10376. It handles refusal paths for user-owned issues.
- Refs #8516. It handles rejected confirmations for user-owned issues.
- Refs #10274. It gives ownerless waking interactions an agent owner.

**What happened?**

An agent created a confirmation on an issue that was assigned to that
same agent and had status `in_review`. A board user accepted the
confirmation. Paperclip started a continuation worker, but the issue
stayed `in_review`. The normal checkout fields stayed empty.

**Expected behavior**

Paperclip must return the issue to an actionable state before it wakes
the continuation worker. The worker must then use atomic checkout to
move the issue to `in_progress`.

**Steps to reproduce**

1. Assign an issue to an agent and set the issue status to `in_review`.
2. Let that agent create a `request_confirmation` with
`wake_assignee_on_accept`.
3. Accept the confirmation as a board user.
4. Observe that the continuation worker starts while the issue remains
`in_review`.

**Paperclip version or commit**

The bug reproduced on master before this pull request. This branch is
based on `ffd62a4cbb`.

**Deployment mode**

Local development. The server logic is deployment-independent.

**Agent adapter(s) involved**

Codex exposed the bug, but the issue-thread continuation logic is
adapter-independent.

**Database mode**

The regression test uses embedded PostgreSQL. The logic is database-mode
independent.

**Access context**

An agent creates the confirmation. A board user accepts it.

## What Changed

- Allow an accepted agent-authored confirmation to return an agent-owned
issue only when the issue is `in_review` and the owner is the creating
agent.
- Keep active `in_progress` work unchanged so an accepted confirmation
cannot reset a running worker to `todo`.
- Add embedded-PostgreSQL regression coverage for user-owned review,
creator-owned review, and creator-owned active work.

## Verification

- `pnpm --filter @paperclipai/server exec vitest run
src/__tests__/issue-thread-interactions-service.test.ts --config
vitest.config.ts` — 48 passed.
- `pnpm -r typecheck` — passed for all workspace projects.
- `pnpm build` — passed for all workspace projects.
- `pnpm test:run` — 3,411 passed. Three timing-sensitive assertions
failed in the unchanged `heartbeat-workspace-busy.test.ts` suite.
- Isolated rerun of `heartbeat-workspace-busy.test.ts` — 15 passed.

## Risks

Low risk. The behavior change is limited to accepted confirmations on
non-terminal `in_review` issues that the creating agent already owns. It
does not change active work, blocked work, terminal issues, other agent
owners, schemas, or public API contracts.

> This is a focused bug fix. It does not add roadmap scope.

## Model Used

OpenAI Codex based on GPT-5. The runtime does not expose the exact
deployment ID or context-window size. The model used reasoning,
repository tools, code editing, Git, and local test execution.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>
@sunnystroeer
sunnystroeer force-pushed the sfs-100-fix-reject-wake branch from d550ba7 to 6dc5e5c Compare August 8, 2026 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

request_confirmation reject on a user-assigned issue never wakes the creator agent (silent stall)

1 participant